Refine error recovery in the parser around top level statements#65262
Refine error recovery in the parser around top level statements#65262AlekseyTs merged 3 commits intodotnet:mainfrom
Conversation
|
@dotnet/roslyn-compiler Please review |
3 similar comments
|
@dotnet/roslyn-compiler Please review |
|
@dotnet/roslyn-compiler Please review |
|
@dotnet/roslyn-compiler Please review |
|
|
||
| [Fact] | ||
| [Trait("Feature", "Directives")] | ||
| public void TestNegIfEndifDirectivesWithBadCode() |
There was a problem hiding this comment.
Why was this test deleted? #Resolved
There was a problem hiding this comment.
Why was this test deleted?
It looked like a dupe of a same named test in TopLevelStatementsParsingTests.cs, but I now realized that directives are not verified there. I will restore this aspect of the test.
| [Fact] | ||
| public void ErrorRecovery_05() | ||
| { | ||
| var test = @"using aliasY = X.Y;"; |
There was a problem hiding this comment.
What's actually erroneous about this syntax? #Resolved
There was a problem hiding this comment.
What's actually erroneous about this syntax?
Nothing, as the test indicates. One of the parts about this change is to avoid recovering from non-error scenarios. That is why some new tests are covering success cases, but it is still an error recovery testing. Negative tests, so to speak.
| { | ||
| var test = @" | ||
| using X; | ||
| using aliasY = X.Y; |
There was a problem hiding this comment.
This one too. I'm not sure why these tests are named ErrorRecovery_X. #Resolved
There was a problem hiding this comment.
Same response as for ErrorRecovery_05. The tests are testing code responsible for an error recovery. Both when it should and shouldn't occur.
| public void ErrorRecovery_09() | ||
| { | ||
| var test = @" | ||
| record class Point(int x, int y); |
There was a problem hiding this comment.
What's an error about this syntax? #Resolved
There was a problem hiding this comment.
What's an error about this syntax?
Same response as for ErrorRecovery_05
|
@dotnet/roslyn-compiler For the second review. |
|
@dotnet/roslyn-compiler For the second review. |
|
|
||
| if (!haveModifiers) | ||
| { | ||
| var resetOnFailurePoint = this.GetResetPoint(); |
There was a problem hiding this comment.
nit: this method is quite long. Can we extra this new block into a new tryParseStatement helper?
jcouv
left a comment
jcouv
left a comment
There was a problem hiding this comment.
LGTM Thanks (iteration 2) with a nit
ghost
modified the milestones:
5 participants
Fixes #55969.